Add function level trace #3633

ksew1 · 2025-08-12T10:04:49Z

Stack:

⚠️ Part of a stack created by spr. Do not merge manually using the UI - doing so may have unexpected results.

commit-id:d194aae4 --- **Stack**: - #3634 - #3633 - #3621 - #3620 ⬅ ⚠️ *Part of a stack created by [spr](https://github.com/ejoffe/spr). Do not merge manually using the UI - doing so may have unexpected results.*

franciszekjob · 2025-08-19T08:13:57Z

crates/debugging/src/trace/function/node.rs

+    pub value: FunctionName,
+    pub children: Vec<FunctionNode>,


Imo it's reasonable to make it private and expose with getters if needed.

franciszekjob · 2025-08-19T08:21:31Z

crates/debugging/Cargo.toml

 serde_json.workspace = true
 starknet-types-core.workspace = true
 starknet.workspace = true
 starknet_api.workspace = true
-thiserror.workspace = true
+thiserror.workspace = true


Newline 😅

franciszekjob · 2025-08-19T08:29:50Z

crates/debugging/src/trace/function/trace.rs

+
+/// Builds a [`FunctionTrace`] from the given stacks of function names.
+fn build_function_trace(stacks: Vec<Vec<FunctionName>>) -> FunctionTrace {
+    let mut dummy_root = FunctionNode::new(FunctionName::NonInlined("root".to_string()));


nit: dummy_root works, but placeholder_root might better convey that it's a temporary node for building the tree:

Suggested change

let mut dummy_root = FunctionNode::new(FunctionName::NonInlined("root".to_string()));

let mut placeholder_root = FunctionNode::new(FunctionName::NonInlined("root".to_string()));

commit-id:6dc6b503 --- **Stack**: - #3634 - #3633 - #3621 ⬅ ⚠️ *Part of a stack created by [spr](https://github.com/ejoffe/spr). Do not merge manually using the UI - doing so may have unexpected results.*

cptartur · 2025-08-19T07:42:05Z

crates/debugging/src/trace/function/name.rs

+#[derive(Debug, Clone, Hash, Eq, PartialEq)]
+pub enum FunctionName {
+    NonInlined(String),
+    // TODO: Add inlined variant in next PR.


Gotta love stacks 🙏 🙏 🙏

cptartur · 2025-08-19T07:44:01Z

crates/debugging/src/trace/function/name.rs

+        Regex::new(r"\[expr\d*]")
+            .expect("Failed to create regex for normalizing loop function names")


Shouldn't we have the regex as a const/static and reuse it instead of creating it multiple times?

cptartur · 2025-08-19T07:44:08Z

crates/debugging/src/trace/function/name.rs

+/// Remove parameters from monomorphised Cairo generics e.g. `<felt252>`.
+fn remove_monomorphization_suffix(function_name: &str) -> String {
+    static RE_MONOMORPHIZATION: LazyLock<Regex> = LazyLock::new(|| {
+        Regex::new(r"::<.*>")


cptartur · 2025-08-19T07:48:31Z

crates/debugging/src/trace/function/node.rs

+            if let Some(child) = self.children.iter_mut().find(|c| c.value == next) {
+                child.add_path_recursive(iter);


Not sure I get it, why do we have this logic?

cptartur · 2025-08-19T07:49:40Z

crates/debugging/src/trace/function/stack.rs

+    /// Enters a new function call by updating the stack with the new call stack.
+    /// It saves the current stack length to allow returning to it later.
+    ///
+    /// The New call stack is expected to be a prefix of the current stack.


What's ensuring that? Only the user contract?
Would checking that be too expensive?

cptartur · 2025-08-19T07:50:08Z

crates/debugging/src/trace/function/stack.rs

+    }
+
+    /// Exits the current function call by truncating the stack to the previous length.
+    /// If there is no previous length, it does nothing.


Why is that? Shouldn't it truncate the stack entirely in this case?

cptartur · 2025-08-19T07:51:39Z

crates/debugging/src/trace/function/stack.rs

+        let empty_or_different_function = self.stack.last().is_none_or(|current_function| {
+            current_function.function_name() != function_name.function_name()
+        });
+
+        if empty_or_different_function {
+            stack.push(function_name);
+        }


So basically it appends to new function to the stack if it's not already present as last?

cptartur · 2025-08-19T09:08:09Z

crates/debugging/src/trace/function/trace.rs

+
+        let program = &program_artifact.program;
+
+        let sierra_program_registry = ProgramRegistry::<CoreType, CoreLibfunc>::new(program)


Some of these objects, like ProgramRegistry we already create during forge runtime. I wonder if there would be any significant benefit in trying to reuse them?

Same goes for constructing trace

cptartur · 2025-08-19T09:13:20Z

crates/debugging/src/tree/ui/display.rs

+}
+
+impl NodeDisplay for FunctionNode {
+    const TAG: &'static str = "inlined";


Are they always inlined?

commit-id:3a7228bd

ksew1 requested a review from a team as a code owner August 12, 2025 10:04

ksew1 requested review from franciszekjob and MKowalski8 and removed request for a team August 12, 2025 10:04

This was referenced Aug 12, 2025

Add test for function level trace #3634

Open

Use context in debugging #3620

Merged

Add casm debug info to ContractsDataStore #3621

Merged

ksew1 force-pushed the spr/master/3a7228bd branch 2 times, most recently from 8a25e67 to 19afecd Compare August 13, 2025 10:44

ksew1 force-pushed the spr/master/6dc6b503 branch from d88921d to 546c8a7 Compare August 13, 2025 10:44

franciszekjob reviewed Aug 19, 2025

View reviewed changes

ksew1 force-pushed the spr/master/3a7228bd branch from 19afecd to df6c5f0 Compare August 19, 2025 08:23

ksew1 force-pushed the spr/master/6dc6b503 branch from 546c8a7 to 2ba7a7c Compare August 19, 2025 08:23

franciszekjob reviewed Aug 19, 2025

View reviewed changes

Base automatically changed from spr/master/6dc6b503 to master August 19, 2025 08:50

ksew1 force-pushed the spr/master/3a7228bd branch from df6c5f0 to 6bc9621 Compare August 19, 2025 08:58

cptartur reviewed Aug 19, 2025

View reviewed changes

ksew1 force-pushed the spr/master/3a7228bd branch from 6bc9621 to 2d73b2a Compare August 21, 2025 13:24

ksew1 mentioned this pull request Aug 21, 2025

Change NodeDisplay const TAG to fn tag(&self) #3657

Draft

Add function level trace

b329d45

commit-id:3a7228bd

ksew1 force-pushed the spr/master/3a7228bd branch from 2d73b2a to b329d45 Compare August 21, 2025 13:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add function level trace #3633

Add function level trace #3633

Uh oh!

ksew1 commented Aug 12, 2025 •

edited

Loading

Uh oh!

franciszekjob Aug 19, 2025

Uh oh!

franciszekjob Aug 19, 2025

Uh oh!

franciszekjob Aug 19, 2025

Uh oh!

cptartur Aug 19, 2025

Uh oh!

cptartur Aug 19, 2025

Uh oh!

cptartur Aug 19, 2025

Uh oh!

cptartur Aug 19, 2025

Uh oh!

cptartur Aug 19, 2025

Uh oh!

cptartur Aug 19, 2025

Uh oh!

cptartur Aug 19, 2025

Uh oh!

cptartur Aug 19, 2025

Uh oh!

cptartur Aug 19, 2025

Uh oh!

cptartur Aug 19, 2025

Uh oh!

Uh oh!

	let mut dummy_root = FunctionNode::new(FunctionName::NonInlined("root".to_string()));
	let mut placeholder_root = FunctionNode::new(FunctionName::NonInlined("root".to_string()));

		Regex::new(r"\[expr\d*]")
		.expect("Failed to create regex for normalizing loop function names")

		if let Some(child) = self.children.iter_mut().find(\|c\| c.value == next) {
		child.add_path_recursive(iter);


		let program = &program_artifact.program;

		let sierra_program_registry = ProgramRegistry::<CoreType, CoreLibfunc>::new(program)

Add function level trace #3633

Are you sure you want to change the base?

Add function level trace #3633

Uh oh!

Conversation

ksew1 commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ksew1 commented Aug 12, 2025 •

edited

Loading